Beacon-api: broadcast blobs in the event of seen block #13830
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
terencechain
force-pushed
the
broadcast-blobs
branch
3 times, most recently
from
a99bd6f to
50ebcfd
Compare
james-prysm
reviewed
Mar 29, 2024
| @@ -2072,3 +2085,26 @@ func (s *Server) GetDepositSnapshot(w http.ResponseWriter, r *http.Request) { | |||
| }, | |||
| ) | |||
| } | |||
|
|
|||
| func (s *Server) broadcastBlobSidecars(ctx context.Context, sb *eth.GenericSignedBeaconBlock) error { | |||
There was a problem hiding this comment.
is it worth using the generic signed beacon block here?
if you just pass the Blobs and kzgProofs from the argument it'll save the New SignedBeacon block and Get deneb calls ( not sure if that saves anything) but I think you have the data of denebBlock already here
terencechain
force-pushed
the
broadcast-blobs
branch
from
f2c0da4 to
90b253a
Compare
james-prysm
reviewed
Mar 29, 2024
| "index": sc.Index, | ||
| "slot": sc.SignedBlockHeader.Header.Slot, | ||
| "kzgCommitment": fmt.Sprintf("%#x", sc.KzgCommitment), | ||
| }).Info("Broadcasted blob sidecar") |
There was a problem hiding this comment.
just to help future debugging this function only runs for equivocatedblocks, can we add something to notify it for the reader of the logs?
terencechain
force-pushed
the
broadcast-blobs
branch
from
811f0b2 to
c168e79
Compare
james-prysm
previously approved these changes
Mar 29, 2024
terencechain
force-pushed
the
broadcast-blobs
branch
from
c168e79 to
4ac7f02
Compare
rkapka
reviewed
Apr 2, 2024
terencechain
force-pushed
the
broadcast-blobs
branch
from
d38c358 to
52f0dc0
Compare
prestonvanloon
previously approved these changes
Apr 2, 2024
terencechain
force-pushed
the
broadcast-blobs
branch
from
52f0dc0 to
723407a
Compare
This reverts commit 091e77e.
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
terencechain
force-pushed
the
broadcast-blobs
branch
from
c6e7b36 to
416a1e9
Compare
james-prysm
approved these changes
Apr 2, 2024
nisdas
pushed a commit
that referenced
this pull request
Jul 4, 2024
* Beacon-api: broadcast blobs in the event of seen block * Fix parameters * Fix test * Check forkchoice * Ran go format * Revert "Ran go format" This reverts commit 091e77e. * James feedback * Radek's feedback Co-authored-by: Radosław Kapka <rkapka@wp.pl> * Fix bad tests --------- Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #13824
As demanded by some mev-boost relayer configurations, we will broadcast blobs even if the event block of the same slot is observed over p2p. There's no reason to refrain from broadcasting these blobs, considering there is no risk of equivocation.
buildBlobSidecarsfor use across both the gRPC and REST packages.errEquivocatedBlock.errEquivocatedBlock, then broadcast the blob sidecar.